1637977 - ThreadSanitizer: data race [@ emplace<>] vs. [@ isNothing] with memory corruption

Reporter

Description

•

5 years ago

The following testcase crashes on mozilla-central revision 20200514-8af03d77567d (tsan-opt build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

See attachment.

Backtrace:

==================
WARNING: ThreadSanitizer: data race (pid=29033)
  Read of size 1 at 0x55dcc2e73210 by thread T7:
    #0 isNothing /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:457:46 (js+0x13c9661)
    #1 Pointer js/src/new-regexp/regexp-shim.h:183:16 (js+0x13c9661)
    #2 v8::internal::RegExpCaseFolding::IgnoreSet() js/src/new-regexp/special-case.cc:53:14 (js+0x13c9661)
    #3 v8::internal::CharacterRange::AddCaseEquivalents(v8::internal::Isolate*, v8::internal::Zone*, v8::internal::ZoneList<v8::internal::CharacterRange>*, bool) js/src/new-regexp/regexp-compiler-tonode.cc:1175:20 (js+0x122c454)
    #4 MakeCaseIndependent js/src/new-regexp/regexp-compiler.cc:2480:9 (js+0x1258586)
    #5 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitText(v8::internal::TextNode*) js/src/new-regexp/regexp-compiler.cc:3634:11 (js+0x1258586)
    #6 v8::internal::TextNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238907)
    #7 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x1257959)
    #8 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitChoice(v8::internal::ChoiceNode*) js/src/new-regexp/regexp-compiler.cc:3649:7 (js+0x1257959)
    #9 v8::internal::ChoiceNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x12387c7)
    #10 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125762e)
    #11 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAction(v8::internal::ActionNode*) js/src/new-regexp/regexp-compiler.cc:3642:5 (js+0x125762e)
    #12 v8::internal::ActionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238787)
    #13 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125762e)
    #14 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAction(v8::internal::ActionNode*) js/src/new-regexp/regexp-compiler.cc:3642:5 (js+0x125762e)
    #15 v8::internal::ActionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238787)
    #16 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125838e)
    #17 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAssertion(v8::internal::AssertionNode*) js/src/new-regexp/regexp-compiler.cc:3692:5 (js+0x125838e)
    #18 v8::internal::AssertionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x12388c7)
    #19 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125762e)
    #20 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAction(v8::internal::ActionNode*) js/src/new-regexp/regexp-compiler.cc:3642:5 (js+0x125762e)
    #21 v8::internal::ActionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238787)
    #22 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x1244029)
    #23 v8::internal::AnalyzeRegExp(v8::internal::Isolate*, bool, v8::internal::RegExpNode*) js/src/new-regexp/regexp-compiler.cc:3712:12 (js+0x1244029)
    #24 js::irregexp::CompilePattern(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, js::RegExpShared::CodeKind) js/src/new-regexp/RegExpAPI.cpp:514:16 (js+0x1215f75)
    #25 js::RegExpShared::compileIfNecessary(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, js::RegExpShared::CodeKind) js/src/vm/RegExpObject.cpp:1033:12 (js+0x693ba4)
    #26 js::RegExpShared::execute(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*) js/src/vm/RegExpObject.cpp:1048:8 (js+0x693c8c)
    #27 ExecuteRegExpImpl js/src/builtin/RegExp.cpp:148:7 (js+0x30342b)
    #28 ExecuteRegExp(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSString*>, int, js::VectorMatchPairs*) js/src/builtin/RegExp.cpp:1015:7 (js+0x30342b)
    #29 js::RegExpTester(JSContext*, unsigned int, JS::Value*) js/src/builtin/RegExp.cpp:1187:7 (js+0x30311a)
    #30 CallJSNative js/src/vm/Interpreter.cpp:493:13 (js+0x36fa39)
    [...]
    #89 JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) js/src/vm/CompilationAndEvaluation.cpp:411:10 (js+0x4cedf9)
    #90 WorkerMain(WorkerInput*) js/src/shell/js.cpp:4234:5 (js+0x219e91)
    #91 callMain<0> js/src/threading/Thread.h:217:5 (js+0x21a220)
    #92 js::detail::ThreadTrampoline<void (&)(WorkerInput*), WorkerInput*&>::Start(void*) js/src/threading/Thread.h:206:11 (js+0x21a220)

  Previous write of size 1 at 0x55dcc2e73210 by thread T8:
    #0 emplace<> /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:779:11 (js+0x13c9682)
    #1 Pointer js/src/new-regexp/regexp-shim.h:184:14 (js+0x13c9682)
    #2 v8::internal::RegExpCaseFolding::IgnoreSet() js/src/new-regexp/special-case.cc:53:14 (js+0x13c9682)
    #3 v8::internal::CharacterRange::AddCaseEquivalents(v8::internal::Isolate*, v8::internal::Zone*, v8::internal::ZoneList<v8::internal::CharacterRange>*, bool) js/src/new-regexp/regexp-compiler-tonode.cc:1175:20 (js+0x122c454)
    #4 MakeCaseIndependent js/src/new-regexp/regexp-compiler.cc:2480:9 (js+0x1258586)
    #5 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitText(v8::internal::TextNode*) js/src/new-regexp/regexp-compiler.cc:3634:11 (js+0x1258586)
    #6 v8::internal::TextNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238907)
    #7 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x1257959)
    #8 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitChoice(v8::internal::ChoiceNode*) js/src/new-regexp/regexp-compiler.cc:3649:7 (js+0x1257959)
    #9 v8::internal::ChoiceNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x12387c7)
    #10 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125762e)
    #11 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAction(v8::internal::ActionNode*) js/src/new-regexp/regexp-compiler.cc:3642:5 (js+0x125762e)
    #12 v8::internal::ActionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238787)
    #13 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125762e)
    #14 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAction(v8::internal::ActionNode*) js/src/new-regexp/regexp-compiler.cc:3642:5 (js+0x125762e)
    #15 v8::internal::ActionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238787)
    #16 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125838e)
    #17 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAssertion(v8::internal::AssertionNode*) js/src/new-regexp/regexp-compiler.cc:3692:5 (js+0x125838e)
    #18 v8::internal::AssertionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x12388c7)
    #19 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x125762e)
    #20 v8::internal::Analysis<v8::internal::(anonymous namespace)::AssertionPropagator, v8::internal::(anonymous namespace)::EatsAtLeastPropagator>::VisitAction(v8::internal::ActionNode*) js/src/new-regexp/regexp-compiler.cc:3642:5 (js+0x125762e)
    #21 v8::internal::ActionNode::Accept(v8::internal::NodeVisitor*) js/src/new-regexp/regexp-compiler.cc:686:1 (js+0x1238787)
    #22 EnsureAnalyzed js/src/new-regexp/regexp-compiler.cc:3607:11 (js+0x1244029)
    #23 v8::internal::AnalyzeRegExp(v8::internal::Isolate*, bool, v8::internal::RegExpNode*) js/src/new-regexp/regexp-compiler.cc:3712:12 (js+0x1244029)
    #24 js::irregexp::CompilePattern(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, js::RegExpShared::CodeKind) js/src/new-regexp/RegExpAPI.cpp:514:16 (js+0x1215f75)
    #25 js::RegExpShared::compileIfNecessary(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, js::RegExpShared::CodeKind) js/src/vm/RegExpObject.cpp:1033:12 (js+0x693ba4)
    #26 js::RegExpShared::execute(JSContext*, JS::MutableHandle<js::RegExpShared*>, JS::Handle<JSLinearString*>, unsigned long, js::VectorMatchPairs*) js/src/vm/RegExpObject.cpp:1048:8 (js+0x693c8c)
    #27 ExecuteRegExpImpl js/src/builtin/RegExp.cpp:148:7 (js+0x30342b)
    #28 ExecuteRegExp(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSString*>, int, js::VectorMatchPairs*) js/src/builtin/RegExp.cpp:1015:7 (js+0x30342b)
    #29 js::RegExpTester(JSContext*, unsigned int, JS::Value*) js/src/builtin/RegExp.cpp:1187:7 (js+0x30311a)
    #30 CallJSNative js/src/vm/Interpreter.cpp:493:13 (js+0x36fa39)
    [...]
    #89 JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) js/src/vm/CompilationAndEvaluation.cpp:411:10 (js+0x4cedf9)
    #90 WorkerMain(WorkerInput*) js/src/shell/js.cpp:4234:5 (js+0x219e91)
    #91 callMain<0> js/src/threading/Thread.h:217:5 (js+0x21a220)
    #92 js::detail::ThreadTrampoline<void (&)(WorkerInput*), WorkerInput*&>::Start(void*) js/src/threading/Thread.h:206:11 (js+0x21a220)

  Location is global 'v8::internal::RegExpCaseFolding::IgnoreSet()::set' of size 208 at 0x55dcc2e73148 (js+0x0000032ad210)

  Thread T7 (tid=29055, running) created by main thread at:
    #0 pthread_create /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:967:3 (js+0x1756ab)
    #1 js::Thread::create(void* (*)(void*), void*) js/src/threading/posix/PosixThread.cpp:52:7 (js+0x47ab31)
    #2 bool js::Thread::init<void (&)(WorkerInput*), WorkerInput*&>(void (&)(WorkerInput*), WorkerInput*&) js/src/threading/Thread.h:90:12 (js+0x219762)
    #3 EvalInWorker(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:4308:29 (js+0x2067f1)
    [...]
    #14 main js/src/shell/js.cpp:11885:12 (js+0x1ee552)

  Thread T8 (tid=29057, running) created by main thread at:
    #0 pthread_create /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:967:3 (js+0x1756ab)
    #1 js::Thread::create(void* (*)(void*), void*) js/src/threading/posix/PosixThread.cpp:52:7 (js+0x47ab31)
    #2 bool js::Thread::init<void (&)(WorkerInput*), WorkerInput*&>(void (&)(WorkerInput*), WorkerInput*&) js/src/threading/Thread.h:90:12 (js+0x219762)
    #3 EvalInWorker(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:4308:29 (js+0x2067f1)
    [...]
    #14 main js/src/shell/js.cpp:11885:12 (js+0x1ee552)

SUMMARY: ThreadSanitizer: data race /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:457:46 in isNothing
==================

In ASan, this shows up in various ways, including doube-free, heap-use-after-free, heap-buffer-overflow and as a negative-size-param to memmove. In debug builds I managed to trigger

Assertion failure: (run->mRegionsMask[elm] & (1U << bit)) == 0 (Double-free?), at memory/build/mozjemalloc.cpp:2203

The attached testcase reproduces the issue intermittently and is only half-reduced.

Christian Holler (:decoder)

Reporter

Comment 1

•

5 years ago

Attached file Testcase — Details

Ryan VanderMeulen [:RyanVM]

Updated

•

5 years ago

status-firefox76: --- → unaffected

status-firefox77: --- → unaffected

status-firefox-esr68: --- → unaffected

Flags: needinfo?(iireland)

Regressed by: 1634135

BMO Automation

Updated

•

5 years ago

Has Regression Range: --- → yes

Daniel Veditz [:dveditz]

Updated

•

5 years ago

Keywords: csectype-race, csectype-uaf, sec-high

Iain Ireland [:iain]

Assignee

Comment 2

•

5 years ago

Attached file Bug 1637977: Make LazyInstanceImpl threadsafe r=tcampbell — Details

V8's LazyInstance is roughly equivalent to lazy_static! in Rust. It's used here: https://searchfox.org/mozilla-central/source/js/src/new-regexp/special-case.cc#49-54.

The current implementation uses a lazily initialized Maybe<T>, but the initialization can race with offthread parsing. This patch wraps the Maybe in an ExclusiveData.

Phabricator Automation

Updated

•

5 years ago

Assignee: nobody → iireland

Status: NEW → ASSIGNED

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 3

•

5 years ago

https://hg.mozilla.org/integration/autoland/rev/f6baadcf251e77fec42c03adbc743146da8ca4b7
https://hg.mozilla.org/mozilla-central/rev/f6baadcf251e

Group: javascript-core-security → core-security-release

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

status-firefox78: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla78

Iain Ireland [:iain]

Assignee

Updated

•

5 years ago

Flags: needinfo?(iireland)

Brindusa Tot, Desktop QA

Updated

•

5 years ago

Flags: qe-verify-

Whiteboard: [bugmon:update,bisect] → [bugmon:update,bisect][post-critsmash-triage]

BugBot [:suhaib / :marco/ :calixte]

Comment 4

•

5 years ago

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(iireland)

Whiteboard: [bugmon:update,bisect][post-critsmash-triage] → [bugmon:update,bisect][post-critsmash-triage][sec-survey]

Iain Ireland [:iain]

Assignee

Updated

•

5 years ago

Flags: needinfo?(iireland)

Daniel Veditz [:dveditz]

Updated

•

4 years ago

Group: core-security-release

Jason Kratzer [:jkratzer]

Comment 5

•

4 years ago

Bugmon Analysis:
Unable to reproduce bug using build nearest the original: mozilla-central 20200708094217-34fb169ef962.
Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon

Testcase 5 years ago Christian Holler (:decoder) 129.63 KB, text/plain		Details
Bug 1637977: Make LazyInstanceImpl threadsafe r=tcampbell 5 years ago Iain Ireland [:iain] 47 bytes, text/x-phabricator-request		Details \| Review

Bugzilla

ThreadSanitizer: data race [@ emplace<>] vs. [@ isNothing] with memory corruption

Categories

(Core :: JavaScript Engine, defect)

Tracking

()

People

(Reporter: decoder, Assigned: iain)

References

(Blocks 1 open bug, Regression)

Details

(7 keywords, Whiteboard: [bugmon:update,bisect][post-critsmash-triage][sec-survey])

Crash Data

Security

(public)

User Story

Attachments

(2 files)

Description

Comment 1

Updated

Updated

Updated

Comment 2

Updated

Comment 3

Updated

Updated

Comment 4

Updated

Updated

Comment 5

Attachment

General

Description

File Name

Content Type